Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding State Checks for Known Type and Value, and Sensitive Checks #273

Closed
wants to merge 54 commits into from

Conversation

bendbennett
Copy link
Contributor

@bendbennett bendbennett commented Jan 11, 2024

Reference: #266

This PR forms part of the changes outlined in #266, specifically it includes state checks for the following:

  • ExpectKnownValue
  • ExpectKnownOutputValue
  • ExpectKnownOutputValueAtPath
  • ExpectSensitiveValue

The implementation in this PR differs from the way in which checks for known type and value are currently being handled for plan checks, in that the ExpectKnown<Value|OutputValue|OutputValueAtPath> state checks allow for explicitly checking for a null value by using a newly introduced null known value check type. An alternative can be seen in the plan check implementations, where the ExpectKnown<Value|OutputValue|OutputValueAtPath> plan checks return an error if a null value is found, and instead there are explicit checks for null values (e.g., ExpectNullOutputValue).

In this context, a significant difference between tfjson.Plan and tfjson.State is the handling of null output values. In the case of tfjson.Plan, an output that specifies an attribute, for instance, which is null appears in the plan, whereas null output values do not appear instate (refer to Allow null value outputs to be present in json output). As a consequence, there is no way for state checks that leverage tfjson.State to discriminate between output values that are null, and output values that do not exist. However, it is worth noting that output values that reference, for instance, an object that has attributes which are null will appear in tfjson.State.

Together this raises the question of whether we want to:

  • Modify plan checks for ExpectKnown<Value|OutputValue|OutputValueAtPath> to handle detection of nulls (to bring them into line with the state checks for ExpectKnown<Value|OutputValue|OutputValueAtPath> in this PR).
  • Modify state checks for ExpectKnown<Value|OutputValue|OutputValueAtPath> in this PR to align their behaviour with the analogous plan checks (i.e., return an error when null is detected).
  • Depending upon how the known value checks are handled, decide whether we want to remove expect null value plan checks, or add expect null value state checks.

Further Considerations

  • The implementation in this PR runs state checks after "post-apply, post-refresh. Do we want to consider running state checks at any other points? I avoided adding state checks for the "refresh" mode as the refresh command is deprecated, but we could consider this if it is thought to valuable. Should we consider state checks for import?
  • We could also consider adding the equivalent state checks for ExpectEmptyPlan and ExpectNonEmptyPlan if there is possible value in having these built-in checks made available.

…pe and value, and known value type and value (#243)
…ue, ExpectKnownOutputValue and ExpectKnownOutputValueAtPath (#243)
  * Configuring when state checks are executed.
  * Testing that state checks are executed.
@bendbennett bendbennett added the enhancement New feature or request label Jan 11, 2024
@bendbennett bendbennett added this to the v1.7.0 milestone Jan 11, 2024
@bendbennett bendbennett changed the base branch from main to bendbennett/issues-243 January 11, 2024 14:18
Base automatically changed from bendbennett/issues-243 to main January 15, 2024 12:28
@bendbennett bendbennett changed the base branch from main to bendbennett/issues-16 January 15, 2024 16:59
@bendbennett bendbennett changed the base branch from bendbennett/issues-16 to main January 15, 2024 17:00
@bendbennett bendbennett deleted the bendbennett/issues-266 branch January 15, 2024 17:19
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant